fix(odata): stop create-external-entities duplicating suffixed associations (formula1 §50) - #124
Merged
Merged
Conversation
…ations mxcli-formula1 §50. Re-running `create external entities from` added two associations every time, without bound: `season_2`, `season_3`, then `season_4`, `season_5`, and so on. One project had reached `season_15` before anyone noticed — `mx check` clean, every test passing, the only symptom duplicate links in Studio Pro's domain model. Association names are unique per module, so the second entity with a `season` nav property is stored as `season_2`. The dedup looks for an association matching the nav property it is about to import, and `season_2` can never match `season` by name. The index that WOULD have matched it is keyed on RemoteParentNavigationProperty — and the modelsdk reader never read the OData association source back, so that index was always empty on the default engine. The write path set the field and the legacy reader read it; only the default read dropped it, so it survived one save and vanished on the next load. assocFromGen now reads Rest$ODataRemoteAssociationSource back (nav properties, navigability, and the four capability flags). The dedup index moves into indexExistingAssociations so it can be tested directly. Not fixed by stripping a trailing _<n> from names: that would also match a user's genuine `season_2`, and the model already records the true origin. Existing damage is not cleaned up here — those associations are referenced by external-entity access rules, so deleting one raises CE1613.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mxcli-formula1§50, still-open item #1. One commit, rebased ontomainafter #123 merged.The report
Re-running
create external entities fromadded two associations every time and never stopped:That project reached
season_15before anyone noticed — twelve spurious associations, committed.mx checkclean, every test passing. The only symptom is duplicate links in Studio Pro's domain model, which nothing in the toolchain looks at.Root cause, one layer below the report
The reported cause — "the dedup compares association names and the generator computes a fresh suffix before it looks" — is the visible half. It does not explain why the dedup has a nav-property index intended for exactly this case.
Association names are unique per module, so three entities each exposing a
seasonnavigation property giveseason,season_2,season_3. A suffixed association can never match its own nav property by name. The dedup knows that, and keys a second index onRemoteParentNavigationProperty— the field recording which OData nav property an association was generated from.That index was always empty. The modelsdk reader never read the OData association source back. The write path sets the field (
domainmodel_write.go:649), the gen type carries it, and the legacy parser reads it — onlyassocFromGenon the default engine dropped it. So the field survived one save and vanished on the next load, silently degrading the dedup to the name match that cannot work.A field the write path sets and the read path drops is invisible to a grep for the identifier: it appears in three places and is missing from a fourth.
The fix
assocFromGennow readsRest$ODataRemoteAssociationSourceback — both nav properties,Navigability2, and the fourCreatable/Updatableflags. The dedup index moves intoindexExistingAssociationsso it is directly testable.Explicitly not fixed by stripping a trailing
_<n>from association names. That heuristic also matches a user's genuineseason_2, and the model already records the true origin — the correct key exists, it just was not being read.Testing
mdl/backend/modelsdk/association_odata_source_test.goseason_2withRemoteParentNavigationProperty: "season"— the exact shape the re-import failed to recognise. Control (reverting the read) fails on all four fields.mdl/executor/cmd_contract_reimport_test.goseasondomain model, asserting all three parents are recognised as already imported.Control: dropping the nav index fails with "DriverStandings.season is not recognised as already imported — a re-import will create a duplicate with a fresh suffix".
Not addressed here
Existing damage. A project that ran the import N times still carries 2N spurious associations, and they cannot simply be deleted — §50 found that external-entity access rules reference them, so removing one leaves CE1613 "The selected association no longer exists". That needs a repair path, which is its own change.
§50's other half — sub-element id churn (still-open #2). Reproduced exactly, on a 9.24 project:
Mechanism:
assignIDalready no-ops on a non-empty id; the churn is that the write path builds fresh gen elements from the semantic model, so they arrive with empty ids and get minted. 162 call sites.One correction to how §50 frames it, which matters for prioritisation. §50 files this as the cosmetic half, separate from "the one that is not cosmetic". That is not reliably true.
06a9facefixed this same shape for entity attributes, where a fresh id made Mendix's DB synchronizer read "attribute departed + new attribute added" and drop and re-add the column — 11 feeds and 98 articles blanked,mx checkclean throughout. So sub-element id churn is cosmetic for some element types and data-destroying for others; the remaining types need judging individually rather than as one bucket.The precedent for the fix is per-type identity reuse (name→id map from the existing document, reuse for retained elements), in the executor rather than as a global mechanism. Across ~630 ids in a domain model and ~858 in a database connection that is a multi-commit change where a mis-matched id is model corruption, so it is deliberately not started here.
go test ./mdl/... ./sdk/...green;go vetandgofmtclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4
Generated by Claude Code